[4.2-dev] fix(frontend): preserve logical result cardinality - #26868
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
CI follow-up pushed in The previous run had two independent failure classes:
The follow-up fixes both production paths, adds boundary/resume and const-null persistence regressions, and updates only the semantically missing NULL rows. Validation on the final patch:
A separate pre-existing const-null/order shuffle panic found during the extended probe reproduces on the main baseline and is tracked in #26871; it is not part of this PR. |
LeftHandCold
left a comment
There was a problem hiding this comment.
Deep review completed on exact head 0bfa799584b300ea31b4402389ebc148149c706d against exact 4.2-dev base/merge-base 9301a23cc8f7c36aa0791b420aad8ed52e2ad3e6.
No actionable findings.
The cross-layer cardinality closure is complete:
- protocol output,
sentRows, saved-query accounting, persisted block cardinality, andresult_scannow consistently use the batch logical row count; - compact const/const-null vectors are normalized only in an owned persistence view, aligned vectors remain borrowed, the executor-owned batch is never mutated, and partial-duplication plus writer-error paths release every temporary vector;
- malformed flat shapes fail before accounting, while zero-row, first/non-first const, const-null, prepared reuse, and column-pruned saved-result reads retain correct semantics;
- OUTER APPLY now increments logical cardinality for null-extended rows and resumes at the exact next input row when
DefaultBatchSizeis reached, without changing CROSS behavior or table-function lifecycle.
Compatibility and cost are bounded: the existing vector/object format is reused, direct output remains an O(1) row-count lookup, aligned saved batches allocate nothing, and normalization is O(column count) with scalar physical copies only. Q1-Q3 found one cleanup owner per duplicate, no new wait dependency, and no unbounded accumulation; the apply change restores the existing batch-size bound.
Independent validation on this exact head: all focused frontend cardinality/persistence tests and apply OUTER/resume tests were enumerated and passed, git diff --check passed, and current GitHub UT/SCA/coverage/BVT/CodeQL checks are green.
What type of PR is this?
Which issue(s) this PR fixes:
issue #25992
Follow-up to #26843 (the
4.2-devbackport of #26836).What this PR does / why we need it:
After #26843 accepts a physical length-1 const vector as a broadcast input, a
prepared grouped query can correctly produce a projection batch whose logical
row count is greater than its first vector's physical length:
The frontend still derived result cardinality from
bat.Vecs[0].Length().Consequently, it emitted only one MySQL result row for a three-row batch and
also undercounted
sentRows. Saved query results had a second inconsistency:their metadata reported three rows, but object storage and
result_scanpreserved only the first physical row.
This change uses
batch.RowCount(), the logical cardinality propagated by theprojection operator, for protocol output and row accounting. At the query-result
persistence boundary, it validates vector shapes and creates an owned writer
view only when compact const vectors need their logical lengths normalized.
Aligned vectors are borrowed, mismatched const/const-null vectors remain
physically scalar, and the executor-owned input batch is never mutated.
Regression coverage includes flat, const, broadcast const, const-null, malformed
and zero-row batches; allocation/write failure cleanup; real object writer plus
result_scan; SQL-level prepared statement reuse; and the originalGROUP BY ?path.
Validation on the exact
4.2-devbase9301a23cc8:parameter-first executions incorrectly return one row.
EXECUTEreturns 3 rows but the immediateresult_scan(last_query_id())returns 1 row.-count=10: pass.pkg/frontendtest suite: pass.go build -mod=readonly ./pkg/frontend: pass.go vet -mod=readonly ./pkg/frontend: pass.make build: pass.NULL-reuse, and non-first parameters: 26/26 statements pass.
COM_STMT_PREPARE/EXECUTEprobe: parameter-first returns 3 rows,NULL statement reuse returns 3 rows, and the [4.2-dev] fix(hashmap): accept broadcast const iterator vectors #26843 grouped-parameter control
returns 1 row.
Direct output keeps an O(1) cardinality lookup. Saved aligned batches allocate
nothing beyond the existing path. A mismatched const adds an O(column count)
validation and copies only scalar physical storage; no per-row materialization
or global object-writer contract change is introduced.